Conversation
There was a problem hiding this comment.
Hello @mohnjiles, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello! Gemini here, providing a summary of this pull request titled "v2.14.1 for real". Based on the title and the changes included, it appears this PR is preparing or finalizing the v2.14.1 release. The primary functional change addresses a potential issue in the update check logic related to how application and update versions are compared when one or both might lack build metadata hashes. Additionally, the changelog has been updated to include a new section acknowledging project supporters.
Highlights
- Update Logic Fix: Corrected the logic used to determine if an update is available, specifically when comparing versions that may or may not have build metadata hashes. The fix ensures that if the current application version lacks a hash but the available update has one, the update is correctly identified as available.
- Changelog Update: Added a new 'Supporters' section to the
CHANGELOG.mdfile under the v2.14.1 entry, listing Visionary and Pioneer tier patrons. - Minor Formatting: A small, non-functional formatting change was made to the
LatestVersionStringproperty inUpdateChannelCard.cs.
Changelog
- CHANGELOG.md
- Added 'Supporters' section for v2.14.1, listing Visionary and Pioneer patrons.
- StabilityMatrix.Avalonia/Models/UpdateChannelCard.cs
- Fixed update check logic in
IsLatestVersionUpdateableto correctly handle cases where the current app version lacks a build metadata hash but the update has one (lines 48-55). - Minor formatting adjustment to
LatestVersionStringproperty (line 23).
- Fixed update check logic in
- StabilityMatrix.Core/Updater/UpdateHelper.cs
- Corrected the condition in
ValidateUpdateto correctly handle update checks when the current app version lacks a build metadata hash and the update has one (line 280).
- Corrected the condition in
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A version bump is nigh,
Fixing bugs that made us sigh,
With hashes now checked,
Updates are perfected,
Ready for users, oh my!
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request v2.14.1 for real appears to be a patch release focused on fixing issues related to update detection, specifically when dealing with commit hashes in version metadata. The changes are clear, well-targeted, and correctly address the described problems. The update to CHANGELOG.md is also well-formatted and informative.
Overall, the fixes enhance the reliability of the update mechanism. Great job on identifying and resolving these subtle but important issues!
Summary of Findings
- Update Detection Logic Correction: The PR correctly fixes a logical error in
UpdateHelper.csfor validating updates when comparing version metadata hashes. The condition was inverted relative to its documented intent. - Improved Edge Case Handling in Update UI: The PR enhances
UpdateChannelCard.csto correctly identify an update as available if the current application version lacks a metadata hash but the new version has one. This improves the user experience by not missing potential updates. - Changelog Update: The
CHANGELOG.mdhas been updated appropriately for v2.14.1, including a new 'Supporters' section which is a nice addition.
Merge Readiness
The changes in this pull request correctly address important issues in the update detection mechanism. The fixes are well-implemented and improve the overall robustness of the application. The code quality is good, and the changelog is updated accordingly.
This pull request appears ready for merging. As always, ensure any CI checks pass and consider a final review by another team member if required by your process. I am an AI assistant and cannot approve pull requests.
|
|
||
| // Always assume update if (We don't have hash && Update has hash) | ||
| if (string.IsNullOrEmpty(updateHash) && !string.IsNullOrEmpty(appHash)) | ||
| if (string.IsNullOrEmpty(appHash) && !string.IsNullOrEmpty(updateHash)) |
There was a problem hiding this comment.
The change in this conditional statement is a crucial fix. The previous logic string.IsNullOrEmpty(updateHash) && !string.IsNullOrEmpty(appHash) meant an update would be assumed if "Update doesn't have hash AND App has hash".
The new logic string.IsNullOrEmpty(appHash) && !string.IsNullOrEmpty(updateHash) correctly implements the intent described in the comment: "Always assume update if (We don't have hash && Update has hash)".
This correction ensures that if the local application build has no commit hash, but the update manifest specifies a version with a commit hash, the update is correctly identified as available. This is a significant improvement for update reliability.
| // Always assume update if (We don't have hash && Update has hash) | ||
| if (string.IsNullOrEmpty(appHash) && !string.IsNullOrEmpty(updateHash)) | ||
| { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
This new condition correctly handles the scenario where the currently installed application version might lack commit hash metadata (e.g., a local or older build), while the available update does include a hash.
By returning true in this case (string.IsNullOrEmpty(appHash) && !string.IsNullOrEmpty(updateHash)), we ensure that users are prompted to update, which is a safer and more robust approach. This aligns well with the changelog entry: "Fixed updates to versions with commit hash version parts not being recognized when the current version has no commit hash part."
Well done for catching this edge case!
No description provided.